perf: prune explicit zeros at matrix assembly (cut build peak memory)#816
Conversation
Expressions that broadcast against a dense coordinate store one coefficient per pair, most of them structurally zero. Those explicit zeros were carried all the way into `matrices.A` and thus into every solver handoff. `highspy.Highs.addRows` (and the other direct backends' matrix loaders) scale with *stored* nnz, so the handoff spent most of its work describing zeros — e.g. sparse_network(250) stored 1.5M entries for 18k structural nonzeros (98.8% zeros). Prune them once, centrally, in `_stack` via `eliminate_zeros()`, so `A` and `indicator_A` — and hence HiGHS, gurobi, xpress, copt, mosek and the LP/MPS writers — all hand the solver only structural nonzeros. A zero coefficient never changes a constraint, so this is mathematically identical. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Follow-up to #815. That eliminates zeros from the stacked constraint matrix after scipy.vstack has already materialised the full dense-with-zeros block, so the peak allocation (what the CodSpeed memory instrument tracks) is unchanged — only the resting size and solver-ingest cost drop. Prune at the source instead: fold `coeffs != 0` into the valid-entry mask in Constraint._matrix_export_data, so broadcast zeros never enter cols/data/the CSR. Snapshot capture and the matrix build share this path, so they stay consistent. Two ripples handled: - matrices.dual derived active rows from stored nnz (np.diff(indptr)); an all-zero-coefficient row would lose its dual slot once pruned. Derive active rows from row activity via a new ConstraintBase.active_row_mask (also avoids rebuilding the CSR just to count rows). - A zero-coefficient term no longer changes the sparsity pattern, so the persistent warm-start path no longer forces a SPARSITY rebuild for it. Test updated to use a non-zero coefficient; a no-rebuild case added. sparse_network(250): build peak 50 -> 36 MB (~27%), identical solver result. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Merging this PR will improve performance by 29.48%
Performance Changes
Tip Curious why this is faster? Comment Comparing Footnotes
|
FabianHofmann
left a comment
There was a problem hiding this comment.
this looks all very sensible to me and make the structure more aligned with the dense representation of constraints. any blockers? I would merge this
|
@FabianHofmann I dont have one, but I merely filed this because I saw the opportunity. I cant confidently say that I know all possible implications fo this change. |
it definitely make the data model and flow cleaner, so I don't think this is harmful. before the next release I will do a wider test sweep with pypsa models anyway |
Intent placeholder — @FBumann to replace with your own words.
Important
Draft — depends on #815. Merge #815 first, then mark this ready.
This targets
master, so until #815 merges the diff below also includes #815's commit (the_stackeliminate_zeros()). Once #815 is in, GitHub collapses the diff to just this PR's source-level prune.Follow-up that completes the zero-drop story on the memory axis.
Note
The following was generated by AI.
Why a second PR
#815 eliminates zeros from the stacked constraint matrix with
eliminate_zeros(). That runs afterscipy.vstackhas already materialised the full dense-with-zeros block, so the peak allocation — exactly what the CodSpeed memory instrument tracks — is unchanged; only the resting matrix size and the solver-ingest cost drop. That's why the memory job shows no movement even though the handoff is measurably faster.Change
Prune at the source: fold
coeffs != 0into the valid-entry mask inConstraint._matrix_export_data, so broadcast zeros never entercols/data/the CSR at all. Snapshot capture and the matrix build both go through this path, so they stay consistent. #815'seliminate_zeros()stays as cheap safety for the one case a pre-filter can't catch — duplicate variable terms that cancel to zero aftersum_duplicates.Two ripples (both handled)
matrices.dualderived its active rows from stored nnz (np.diff(csr.indptr)). Once zeros are pruned, an all-zero-coefficient row (e.g.0·x ≤ 5) keeps its row inAbut stores no entry, so it would silently lose its dual slot → misaligned dual vector. Fixed by deriving active rows from row activity via a newConstraintBase.active_row_mask(coefficient-independent; also skips a redundant CSR rebuild).SPARSITYrebuild when one is added — correct, since the matrix is unchanged.test_shape_mismatch_triggers_sparsity_rebuildupdated to use a non-zero coefficient (still exercises the real path), andtest_zero_coefficient_term_needs_no_rebuildadded for the new behaviour.Impact
Peak allocation building
m.matrices.A— sparse_network(250)_stackeliminate_zeros)~27% lower peak. The residual is the unavoidable dense-coeffs flatten — removing that needs expression-level sparsity, out of scope here. Solver result is identical;
addMConstr/addRowsstay ~2× faster from the smaller nnz.Full test suite green (3770 passed, 45 skipped).
🤖 Generated with Claude Code